Skip to content

Add a 'Sync now' button to the sync status header in the tray window#5018

Merged
claucambra merged 3 commits into
masterfrom
feature/force-sync-tray-button
Oct 25, 2022
Merged

Add a 'Sync now' button to the sync status header in the tray window#5018
claucambra merged 3 commits into
masterfrom
feature/force-sync-tray-button

Conversation

@claucambra

@claucambra claucambra commented Oct 7, 2022 •
edited
Loading

Copy link
Copy Markdown
Collaborator

image

Closes #4877

Signed-off-by: Claudio Cambra claudio.cambra@gmail.com

@claucambra claucambra self-assigned this Oct 7, 2022
mgallien
mgallien reviewed Oct 12, 2022
View reviewed changes

@mgallien mgallien left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

works well
will be very convenient
but as an user I would never discover this hidden gem
we should have visible on hover feedback

@claucambra

claucambra commented Oct 12, 2022

Copy link
Copy Markdown
Collaborator Author

works well will be very convenient but as an user I would never discover this hidden gem we should have visible on hover feedback

Understood, will add a hover effect

@claucambra claucambra force-pushed the feature/force-sync-tray-button branch from 064b753 to 6a542c1 Compare October 12, 2022 16:52
@claucambra

claucambra commented Oct 12, 2022

Copy link
Copy Markdown
Collaborator Author

works well will be very convenient but as an user I would never discover this hidden gem we should have visible on hover feedback

Done, here's a video:

simplescreenrecorder-2022-10-12_18.51.32.mp4

@claucambra claucambra requested a review from mgallien October 12, 2022 16:53
@claucambra claucambra force-pushed the feature/force-sync-tray-button branch from 6a542c1 to 88a678b Compare October 12, 2022 17:00
@codecov

codecov Bot commented Oct 12, 2022 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #5018 (c0baf5c) into master (6db3361) will decrease coverage by 0.00%.
The diff coverage is 55.55%.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #5018      +/-   ##
==========================================
- Coverage   57.22%   57.21%   -0.01%     
==========================================
  Files         138      138              
  Lines       17444    17441       -3     
==========================================
- Hits         9982     9979       -3     
  Misses       7462     7462
Impacted Files Coverage Δ
src/libsync/owncloudpropagator.h 68.42% <0.00%> (-6.02%) ⬇️
src/libsync/syncengine.h 50.00% <57.14%> (+6.25%) ⬆️
src/libsync/syncengine.cpp 84.16% <100.00%> (-0.08%) ⬇️
src/libsync/vfs/cfapi/cfapiwrapper.cpp 74.09% <0.00%> (-0.23%) ⬇️
src/libsync/propagatedownload.cpp 64.61% <0.00%> (+1.17%) ⬆️

@jancborchardt

jancborchardt commented Oct 20, 2022

Copy link
Copy Markdown
Member

Hm, after looking at the video, thinking about it again, with @mgallien’s feedback, and @Andreas-Kainz’s original issue at #4877, I now agree with the proposal to actually just add a button there. :D

Should be on the right, saying "Sync now", doesn’t need an icon. And then the whole line does not need a hover state, cause that actually looks a bit off.

The button can be text-only or have a simple border around it (with pill-style radius). What do you think @claucambra?

@claucambra claucambra force-pushed the feature/force-sync-tray-button branch from 88a678b to ce858e4 Compare October 20, 2022 11:44
@claucambra

claucambra commented Oct 20, 2022

Copy link
Copy Markdown
Collaborator Author

Hm, after looking at the video, thinking about it again, with @mgallien’s feedback, and @Andreas-Kainz’s original issue at #4877, I now agree with the proposal to actually just add a button there. :D

Should be on the right, saying "Sync now", doesn’t need an icon. And then the whole line does not need a hover state, cause that actually looks a bit off.

The button can be text-only or have a simple border around it (with pill-style radius). What do you think @claucambra?

Like this? :)

image

@claucambra claucambra force-pushed the feature/force-sync-tray-button branch from ce858e4 to 91e6bf4 Compare October 20, 2022 11:54
@claucambra claucambra changed the title Make the sync status clickable to force a sync (if a sync not already running) Add a 'Sync now' button to the sync status header in the tray window Oct 20, 2022
@claucambra claucambra force-pushed the feature/force-sync-tray-button branch from 91e6bf4 to 4669671 Compare October 24, 2022 11:48
@mgallien mgallien force-pushed the feature/force-sync-tray-button branch from 4669671 to 51e4b4f Compare October 24, 2022 16:37
@mgallien

mgallien commented Oct 25, 2022

Copy link
Copy Markdown
Collaborator

@jancborchardt @nimishavijay are you OK with the current state ?

jancborchardt
jancborchardt approved these changes Oct 25, 2022
View reviewed changes

@jancborchardt jancborchardt left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, looks great @claucambra, straight and to the point. :)

Will then for sure take care of cases where people want to absolutely sync stuff right now (before they shut down the device e.g.).

@claucambra claucambra force-pushed the feature/force-sync-tray-button branch from 51e4b4f to 9a25699 Compare October 25, 2022 17:13
@claucambra
Create commonly accessible properties for current user colours in Sty…
1a6326e 
…le.qml

Signed-off-by: Claudio Cambra <claudio.cambra@nextcloud.com>
@claucambra
Add forcefoldersync method to folder manager
94b45d6 
Signed-off-by: Claudio Cambra <claudio.cambra@nextcloud.com>
@claucambra
Add a 'Sync now' button to the sync status header in the tray window
c0baf5c 
Signed-off-by: Claudio Cambra <claudio.cambra@nextcloud.com>
@claucambra claucambra force-pushed the feature/force-sync-tray-button branch from 9a25699 to c0baf5c Compare October 25, 2022 17:14
@sonarqubecloud

sonarqubecloud Bot commented Oct 25, 2022

Copy link
Copy Markdown

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@nextcloud-desktop-bot

nextcloud-desktop-bot commented Oct 25, 2022

Copy link
Copy Markdown

AppImage file: nextcloud-PR-5018-c0baf5c93cdc31349fcb8c4dd339784936faa92b-x86_64.AppImage

To test this change/fix you can simply download above AppImage file and test it.

Please make sure to quit your existing Nextcloud app and backup your data.

@claucambra claucambra merged commit 95898fd into master Oct 25, 2022
@claucambra claucambra deleted the feature/force-sync-tray-button branch October 25, 2022 19:33
@AndyXheli

AndyXheli commented Oct 26, 2022

Copy link
Copy Markdown

#4608

@mgallien mgallien added this to the 3.7.0 milestone Nov 8, 2022
@allexzander

allexzander commented Dec 6, 2022

Copy link
Copy Markdown
Contributor

/backport to stable-3.6

@allexzander allexzander modified the milestones: 3.7.0, 3.6.3 Dec 6, 2022
@backportbot-nextcloud backportbot-nextcloud Bot mentioned this pull request Dec 6, 2022
Closed
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mgallien mgallien mgallien approved these changes
@jancborchardt jancborchardt jancborchardt approved these changes
@camilasan camilasan Awaiting requested review from camilasan camilasan is a code owner
@allexzander allexzander Awaiting requested review from allexzander
@nimishavijay nimishavijay Awaiting requested review from nimishavijay

Assignees

@claucambra claucambra

Labels

None yet

Projects

None yet

Milestone

3.6.3

Development

Successfully merging this pull request may close these issues.

Add start sync button

6 participants

@claucambra @jancborchardt @mgallien @nextcloud-desktop-bot @AndyXheli @allexzander